oze: 未初期化 ScanRange union を修正 + -Werror=maybe-uninitialized を有効化 - #44
Merged
Conversation
…alized Phase 1 of the phased -Werror cleanup tracked in #43. Promotes one warning class to error at a time and fixes its offenders in the same PR. The fix ======= cc/oze/include/transaction.hh:228 declared `uint64_t updated;` without an initializer and then only `|=`'d into the upper or lower 32 bits in two separate if/else branches. The other 32 bits stayed garbage, so the value written back to ScanRange[index] via compare_exchange_strong was not the intended `union(current [min,max], new [left,right])`. Why this is an actual bug, not just a hygiene warning ----------------------------------------------------- ScanRange is the fast-path bounding box read by the insert-validation path at cc/oze/transaction.cc:851 — it splits the 64-bit atomic into upper-32 = min, lower-32 = max and short-circuits the ScanHistory linear walk if the inserted key falls outside [min, max]. Maintaining that bounding box correctly requires it to grow monotonically: new_min = min(current_min, left) new_max = max(current_max, right) scan_range = (new_min << 32) | new_max With `updated` uninitialized, the OR with garbage can: - shrink the apparent min and grow the apparent max, masking inserts that should have triggered the ScanHistory check (correctness: potential phantom anomaly), or - skew the range so the fast-path never short-circuits (perf only). Either way, the behavior depends on stack contents, which is exactly what -Wmaybe-uninitialized flagged. Initializing `updated = 0` lets the two `|=` branches build the intended `(min << 32) | max` layout. Paper cross-check ----------------- The Oze PVLDB paper (Nemoto et al., PVLDB vol.18 p2321; extended version at arxiv:2210.04179) describes phantom prevention via a per- transaction scan history (txid + predicates) checked by inserters in the validation phase. ScanRange is not in the paper — it is an implementation-side fast-path filter in front of that scan history. The fix preserves the paper's correctness contract (ScanHistory is unchanged) while making the fast-path do what the surrounding code clearly intends. CMake side ========== - `ccbench_add_protocol()` now adds `-Werror=maybe-uninitialized` to every protocol target. This is the first promotion in #43's phased rollout; the comment about deferring -Werror is replaced with one explaining the phased-promotion pattern. - Verified: Debug+ASan and Release both build all 34 binaries clean.
The CI gcc (13.x) flagged cc/cicada/transaction.cc:489 even though the runtime path that reads pre_ver via compare_exchange_strong is only reachable after the while-loop has assigned to it. GCC 13's flow analysis can't prove that across the four-way condition guarding the read, so it warns. Initialize to nullptr to make the false positive go away — no semantic change. Local gcc 11.4 did not catch this, so the previous PR turned the build red on CI only.
Same pattern as the cicada fix: GCC 13 cannot prove that `threshold` is always written before the RLL_ loop reads it on line 757. At runtime it is — either by the per-violation assignment in the CLL_ scan or by the explicit `if (vioctr == 0) threshold = (Tuple*)-1` afterwards — but the static analysis gives up. Initialize at declaration with the same sentinel (max pointer) the explicit guard would set, and leave the existing guard in place so the intent of the default value stays visible at its original site.
…padding LogRecord::computeChkSum() casts `this` to `int*` and sums every int chunk of the object — including any trailing struct padding after val_[VAL_SIZE]. On GCC 13 that padding triggers -Werror=maybe-uninitialized at silo/transaction.cc:447 where a LogRecord is constructed. Zero the entire object in both constructors before assigning members. std::string_view is trivially copyable, so a memset-then-assign sequence is well-defined for that member.
This PR turned CI red three times in a row (cicada pre_ver, mocc threshold, silo LogRecord padding) because the devcontainer's GCC 11 is more permissive about -Wmaybe-uninitialized than CI's GCC 13. Each round was a one-line fix surfaced only by CI, which is the wrong loop: cheap to verify locally, expensive to spin CI runners for. Document the mismatch and the two reasonable workflows (PPA install of gcc-13, or a one-shot ubuntu:24.04 docker run), and call out that multiple consecutive CI fixups for a single warning promotion is a signal you skipped this step.
This was referenced May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#43 Phase 1 のサブタスク。
-Wmaybe-uninitialized(1 件) を潰し、同 PR でccbench_add_protocol()に-Werror=maybe-uninitializedを追加する。修正内容
cc/oze/include/transaction.hh:228でuint64_t updated;が未初期化のまま宣言され、その下の if/else で 上位 32 bit または下位 32 bit だけに|=していた。残り 32 bit はスタックのガベージのまま compare_exchange で書き戻されていた。なぜ単なる hygiene warning ではなく実バグなのか
ScanRangeはcc/oze/transaction.cc:851の insert-validation で 挿入キーが scan された bounding box に入るか をチェックする fast-path として使われている (upper-32 = min,lower-32 = max)。これが monotonically growing であることが前提:updatedがガベージのままだと:minが実値より小さく、maxが実値より大きく見えるパターンでは fast-path は通過するが OK。だが逆に見かけのminがガベージで大きくなり、見かけのmaxがガベージで小さくなるパターンではprefix < min || max < prefixの判定が誤って真になり、本来 ScanHistory チェックすべき挿入がスキップされて phantom anomaly につながる可能性いずれもスタック内容依存。
-Wmaybe-uninitializedがまさにこれを拾った。論文との整合性
Oze PVLDB 論文 (Nemoto et al., PVLDB vol.18 p2321 / extended version arXiv:2210.04179) は phantom 防止を scan history (txid + predicates) を挿入側が validation phase でチェック することで実現すると記述。論文に
ScanRangeの記述はない — 実装側でScanHistoryの線形走査の前段に置かれた bounding-box フィルタ に相当する性能最適化。論文の正当性契約 (ScanHistoryのロジック) は本修正で変更していない。CMake 側
ccbench_add_protocol()に以下を追加:これが #43 ロードマップの最初の promotion。以後の Phase で同様のパターンで一個ずつ
-Werror=<flag>を足していく。Test plan
rm -rf build && cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build -jで 34/34 ビルド成功cmake -S . -B build-release -DCMAKE_BUILD_TYPE=Release -DENABLE_SANITIZER=OFF && cmake --build build-release -jで 34/34 ビルド成功